• lightning/src/ln/blinded_payment_tests.rs (4 hunks)
• lightning/src/ln/onion_utils.rs (9 hunks)
• lightning/src/ln/outbound_payment.rs (5 hunks)
• lightning/src/routing/router.rs (9 hunks)

lightning/src/ln/blinded_payment_tests.rs (4)

• 13-13: The addition of PaymentFailureReason to the events module is a good practice as it allows for more descriptive error handling in the payment process.

• 59-78: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [30-66]

The refactoring of the blinded_payment_path function to return a tuple of BlindedPayInfo and BlindedPath is a significant change. Ensure that all usages of this function have been updated to handle the new return type.

• 68-75: The get_blinded_route_parameters function has been modified to use the refactored blinded_payment_path function. This change should be verified to ensure that the RouteParameters are constructed correctly with the new return type of blinded_payment_path.

Verification successful

---

The get_blinded_route_parameters function correctly uses the tuple returned by blinded_payment_path to construct RouteParameters. The blinded_payment_path function returns a tuple (BlindedPayInfo, BlindedPath), which is then passed into PaymentParameters::blinded within a vec![] to create the RouteParameters.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Verify that `get_blinded_route_parameters` correctly constructs `RouteParameters` using the new `blinded_payment_path` return type.
ast-grep --lang rust --pattern $'get_blinded_route_parameters($_, $_, $_, $_, $_)'

Length of output: 2489

---

---

Script:

#!/bin/bash
# Check the implementation of `get_blinded_route_parameters` and the return type of `blinded_payment_path`
rg "fn get_blinded_route_parameters" lightning/src/ln/blinded_payment_tests.rs
rg "fn blinded_payment_path" lightning/src/ln/blinded_payment_tests.rs

Length of output: 215

---

---

Script:

#!/bin/bash
# Retrieve the full implementation of `get_blinded_route_parameters` and `blinded_payment_path` to verify the return type and usage.
rg -A 10 "fn get_blinded_route_parameters" lightning/src/ln/blinded_payment_tests.rs
rg -A 10 "fn blinded_payment_path" lightning/src/ln/blinded_payment_tests.rs

Length of output: 1061

* 712-815: The new test `blinded_path_retries` is added to verify the retry behavior of blinded payment paths. It's important to ensure that this test covers all relevant scenarios, including the failure and retry mechanisms. The test seems comprehensive, but it's crucial to check that it fails when expected and that the retry logic is sound.

Verification successful

---

The blinded_path_retries test function has been successfully located and its contents displayed. The test sets up a network topology, creates channels, and simulates a payment process with retries over blinded paths. It includes assertions to check the correct behavior when a payment fails and when all blinded paths have been exhausted.

The test appears to cover:

• The setup of a network with a specific topology for testing blinded paths.
• The creation of a payment with blinded route hints.
• The simulation of a payment failure through a blinded path.
• The handling of a failed payment and the processing of the retry logic.
• The assertion that the payment fails permanently when all blinded paths have been tried and no more retries are available.

Based on the provided output, the test seems to be comprehensive in terms of testing the retry behavior of blinded payment paths, including the failure and retry mechanisms. It checks for both temporary failure (allowing for a retry) and permanent failure (when no more retries are possible).

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Verify that the new test `blinded_path_retries` is comprehensive and covers all relevant scenarios.
ast-grep --lang rust --pattern $'fn blinded_path_retries()'

Length of output: 60

---

---

Script:

#!/bin/bash
# Adjusting the pattern to match the function definition correctly.
ast-grep --lang rust --pattern $'fn blinded_path_retries($_) { $$$ }'

Length of output: 70

---

---

Script:

#!/bin/bash
# Using rg to search for the function definition within the file.
rg "fn blinded_path_retries"

Length of output: 100

---

---

Script:

#!/bin/bash
# Display the contents of the `blinded_path_retries` function to examine its comprehensiveness.
cat lightning/src/ln/blinded_payment_tests.rs | sed -n '/fn blinded_path_retries()/,/^}/p'

Length of output: 4727

lightning/src/ln/onion_utils.rs (10)

• 432-432: The addition of the failed_within_blinded_path field to the DecodedOnionFailure struct is consistent with the PR's objective to track failures within blinded paths.

• 467-467: The failed_within_blinded_path field is correctly initialized within the FailureLearnings struct, which is used to capture information about the failure for further processing.

• 493-494: The failed_within_blinded_path field is correctly set to true when an error is detected from within a blinded route, which aligns with the PR's goal.

• 526-527: Again, the failed_within_blinded_path field is set to true when the failing hop is within a multi-hop blinded path, which is consistent with the intended logic.

• 557-558: The failed_within_blinded_path field is set to false when the error is not related to a blinded path, which is correct behavior.

• 714-715: The failed_within_blinded_path field is correctly assigned to the DecodedOnionFailure struct based on the FailureLearnings struct, ensuring that the information about the failure is preserved.

• 729-729: The failed_within_blinded_path field is correctly included in the construction of the DecodedOnionFailure struct, which is the final step in processing the onion failure.

• 740-740: The failed_within_blinded_path field is set to false by default when no specific failure learnings are available, which is a safe default.

• 888-888: The failed_within_blinded_path field is correctly set to false when decoding a failure from the first hop, as this is not a blinded path scenario.

• 723-732: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [432-888]

The changes made to the DecodedOnionFailure struct and the process_onion_failure function are consistent with the PR's objectives to enhance the reliability of blinded routes by tracking and avoiding failed paths. The failed_within_blinded_path field has been correctly added and utilized throughout the relevant functions to ensure that once a blinded path fails, it is not retried in subsequent payment attempts.

lightning/src/ln/outbound_payment.rs (6)

• 22-22: The import of BlindedTail on line 22 suggests that there are changes related to blinded paths, which aligns with the PR's objectives.

• 132-136: The method insert_previously_failed_blinded_path on lines 132-136 is a new addition that allows marking a blinded path as failed. This is a key part of the PR's functionality to avoid retrying failed blinded paths.

• 1612-1617: The code on lines 1612-1617 is under a cfg(test) flag, which means it's only compiled for testing purposes. It's important to ensure that the test code is consistent with the production code logic. The DecodedOnionFailure struct is being used here, which is relevant to the handling of onion routing failures.

• 1656-1661: Lines 1656-1661 handle the case where a failure occurs within a blinded path. This is a new check introduced in this PR, and it's crucial for the new functionality to track failures in blinded paths correctly.

• 1612-1617: The use of cfg(test) on lines 1612-1617 indicates that the following code is only for testing purposes. It's important to ensure that the test code is consistent with the production code logic. The DecodedOnionFailure struct is being used here, which is relevant to the handling of onion routing failures.

• 1656-1661: Lines 1656-1661 handle the case where a failure occurs within a blinded path. This is a new check introduced in this PR, and it's crucial for the new functionality to track failures in blinded paths correctly.

lightning/src/routing/router.rs (8)

• 715-718: The addition of previously_failed_blinded_path_idxs to PaymentParameters is consistent with the PR's objective to track failed blinded paths. Ensure that the type u64 is appropriate for indexing and that there are no potential overflows or underflows when casting indices to this type.

• 740-740: In the Writeable implementation for PaymentParameters, the new field previously_failed_blinded_path_idxs is serialized with a tag of 11. Verify that this tag does not conflict with any existing serialization tags and that it follows the serialization format's guidelines.

Verification successful

---

The executed script was intended to search for the use of tag 11 in the serialization code within router.rs. The absence of output from the rg command suggests that there are no other instances of tag 11 being used in the Writeable implementation for PaymentParameters or elsewhere in the router.rs file. This indicates that the tag 11 does not conflict with any existing serialization tags within that file.

Based on the information gathered, it can be concluded that the use of tag 11 for the field previously_failed_blinded_path_idxs in the Writeable implementation for PaymentParameters does not conflict with any other serialization tags in the router.rs file.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Verify that tag 11 is not used by other fields in PaymentParameters serialization
rg "write_tlv!.*\(11," src/routing/router.rs

Length of output: 108

---

---

Script:

#!/bin/bash
# First, find the correct path for `router.rs`
fd router.rs

# If the file exists, then search for the use of tag 11 in the serialization within `router.rs`
# This command will be executed only if `router.rs` is found
fd router.rs --exec rg "write_tlv!.*\(11,"

Length of output: 110

---

---

Script:

#!/bin/bash
# Directly search for the use of tag 11 in the serialization within `router.rs`
rg "write_tlv!.*\(11," lightning/src/routing/router.rs

Length of output: 56

* 759-759: In the `ReadableArgs` implementation, `previously_failed_blinded_path_idxs` is marked as optional. This is a good approach for backward compatibility. However, ensure that the rest of the code correctly handles the case when this vector is not present in the data.

• 783-783: The default value for previously_failed_blinded_path_idxs is an empty vector, which is appropriate. Confirm that there are no scenarios where a non-empty default would be necessary.

• 802-802: The initialization of previously_failed_blinded_path_idxs with an empty vector in the new and with_max_channel_saturation_power_of_half methods is consistent and ensures that the field is properly initialized in all constructors.

Also applies to: 841-841

• 918-929: The insert_previously_failed_blinded_path method adds an index to previously_failed_blinded_path_idxs. Ensure that there is logic in place to prevent duplicate indices from being added, as this could lead to inefficiencies or unexpected behavior.

• 1386-1394: The blinded_hint_idx method provides a way to retrieve the index of a blinded hint. This is a utility method that will likely be used in multiple places. Confirm that the method is used consistently and that the returned index is always valid.

• 2146-2154: The logic to check if a payment failed on a specific channel or blinded path is correct. It uses the new fields to determine if a channel or blinded path index should be avoided. Ensure that the unwrap call on line 2150 is safe and will not cause a panic in any scenario.

Verification successful

---

The review comment has been verified. The unwrap() call on line 2150 in router.rs is preceded by a check that ensures it is safe to use, as it is only called if blinded_hint_idx().is_some() returns true. This means the unwrap() will not cause a panic in this scenario.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Check for all usages of blinded_hint_idx to ensure it's always called when Some
rg "blinded_hint_idx\(\)" src/routing/router.rs

Length of output: 111

---

---

Script:

#!/bin/bash
# First, find the correct path for the router.rs file
fd 'router.rs'

# If the file is found, then search for the usages of blinded_hint_idx()
# The output of fd will be used to ensure the correct file path is used

Length of output: 65

---

---

Script:

#!/bin/bash
# Now that we have the correct path, search for the usages of blinded_hint_idx() in router.rs
rg "blinded_hint_idx\(\)" lightning/src/routing/router.rs

Length of output: 175

• lightning/src/ln/blinded_payment_tests.rs (4 hunks)
• lightning/src/ln/onion_utils.rs (9 hunks)
• lightning/src/ln/outbound_payment.rs (5 hunks)
• lightning/src/routing/router.rs (9 hunks)

• lightning/src/ln/blinded_payment_tests.rs
• lightning/src/ln/onion_utils.rs
• lightning/src/ln/outbound_payment.rs
• lightning/src/routing/router.rs